Skip to content

UNDERTOW-1794 DefaultAccessLogReceiver violates Closeable contract #1593

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 2 commits into
base: 2.2.x
Choose a base branch
from
Open

UNDERTOW-1794 DefaultAccessLogReceiver violates Closeable contract #1593

wants to merge 2 commits into from

Conversation

@baranowb
Copy link
Contributor

@baranowb baranowb commented May 14, 2024 •
edited
Loading

@baranowb baranowb force-pushed the UNDERTOW-1794_2.2.x branch from db0351a to ace463d Compare July 10, 2024 07:03
@ropalka ropalka requested review from fl4via and ropalka August 6, 2024 17:39
@baranowb baranowb added enhancement Enhances existing behaviour or code new feature/API change New feature to be introduced or a change to the API (non suitable to minor releases) waiting peer review PRs that edit core classes might require an extra review labels Sep 4, 2024
@fl4via fl4via added the under verification Currently being verified (running tests, reviewing) before posting a review to contributor label Oct 21, 2024
jasondlee
jasondlee reviewed Nov 26, 2024
View reviewed changes
core/src/main/java/io/undertow/server/handlers/accesslog/DefaultAccessLogReceiver.java
Comment on lines +229 to +230
if(bw != null)
bw.flush();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this block have curlies? Not sure what any coding styles for Undertow might say, but I find they're safer to have.

jasondlee
jasondlee approved these changes Nov 26, 2024
View reviewed changes
Copy link
Contributor

@jasondlee jasondlee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than a very minor style comment/question, this looks good to me.

This was referenced Oct 22, 2025
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasondlee jasondlee jasondlee approved these changes

@ropalka ropalka Awaiting requested review from ropalka

@fl4via fl4via Awaiting requested review from fl4via

Assignees

No one assigned

Labels

enhancement Enhances existing behaviour or code new feature/API change New feature to be introduced or a change to the API (non suitable to minor releases) under verification Currently being verified (running tests, reviewing) before posting a review to contributor waiting peer review PRs that edit core classes might require an extra review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@baranowb @jasondlee @fl4via